ledger: increase locks granularity in lookupWithoutRewards #5637
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5637 +/- ##
=======================================
Coverage 54.94% 54.94%
=======================================
Files 463 463
Lines 64476 64485 +9
=======================================
+ Hits 35427 35433 +6
+ Misses 26668 26667 -1
- Partials 2381 2385 +4
... and 6 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| // there is a chance the db advanced between deltas check and the baseAccounts check | ||
| // ensure the round requested is still in the db | ||
| if macct.Round > rnd { | ||
| return ledgercore.AccountData{}, 0, "", 0, &RoundOffsetError{rnd, macct.Round} |
There was a problem hiding this comment.
Why do you think it is worth taking an optimistic (instead of preventing an error here with locks), and returning an error when the database advances?
There are operation that need to get canceled because an error is returned from here.
There was a problem hiding this comment.
This is only actual for rest api since txpool and block validator need the latest state of account to validate.
|
Looks reasonable to me. The only suggestion is, consider moving the two |
Summary
This PR is a rework of #5527 (later revered in #5620 because of test failure).
Why does it work:
say au.deltas has 10 elements that is backed by some array underneath. e make local
deltas = au.deltas[0:len(au.deltas)], pointing to the same backing array. Then there are two options:deltaskeeps pointing on itdeltaskeeps pointing to the old backing arrayau.deltaskeeps pointing to the same backing array but to the right subset, ourdeltasis fine as wellThe change would benefit from @icorderi work on making lru caches concurrency-safe and possible switch to a bound circular buffer of deltas.
Why is it safe
lookupWithoutRewardsis used by block evaluator in txpool, ensureBlock, catchup to add a new block into the ledger, i.e. always hasrnd=latest. Having theMaxAcctLookback > 1(deltas size, actual value is 4) guarantees the requestedrndis greater than the flushed rounds, and forrnd=latestthe invariant "value is in deltas, or is in baseAccounts if flushed recently, or in DB" still holds.Test Plan
Existing tests should pass